FIX: warn instead of raising on reasoning-model token truncation#2166
FIX: warn instead of raising on reasoning-model token truncation#2166romanlutz wants to merge 6 commits into
Conversation
When a reasoning model hits max_completion_tokens, the API returns finish_reason='length' with empty visible content. Previously this raised EmptyResponseException (misleadingly reported as 'Status Code: 204') and triggered the 10x retry storm. _validate_response now logs a warning explaining that reasoning models consume tokens on hidden reasoning, and returns a graceful empty response (error='empty') instead of raising, so runs continue. Non-length empty responses still raise as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…se target Mirror the OpenAIChatTarget truncation fix in OpenAIResponseTarget. When the Responses API returns status='incomplete' with reason='max_output_tokens', warn and preserve any completed output (reasoning, partial text) instead of raising a PyritException that halts the run. Falls back to a graceful empty text piece when no visible text was produced, and skips partial tool/function-call sections so an incomplete call cannot re-enter the agentic loop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00b73356-ae65-4d83-8220-ebad37cc5850
…construction Address review feedback: _validate_response no longer returns a Message. It now only validates (warns and returns None on token-limit truncation, raises on genuinely empty responses so retries still fire). Building the graceful-empty or partial truncated Message moves into _construct_message_from_response_async in both OpenAIChatTarget and OpenAIResponseTarget, and the shared _validate_response return type narrows to None. Behavior is unchanged; responsibilities are cleaner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00b73356-ae65-4d83-8220-ebad37cc5850
Reconcile the reasoning-model truncation fix with main's extraction of shared Chat Completions response parsing. The chat target keeps its truncation policy (warn on finish_reason='length', graceful empty on no content) layered over the shared validate/build helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00b73356-ae65-4d83-8220-ebad37cc5850
| choice = response.choices[0] if getattr(response, "choices", None) else None | ||
| if choice is not None and getattr(choice, "finish_reason", None) == "length": |
There was a problem hiding this comment.
It would be nice if we can move this to its own function, e.g. get_finish_reason(response=response) . Makes the logic reusable and makes this line more readable.
| choice = response.choices[0] if getattr(response, "choices", None) else None | ||
| if choice is not None and getattr(choice, "finish_reason", None) == "length": |
There was a problem hiding this comment.
Circling back to my comment above, this should be its own function
There was a problem hiding this comment.
Can we add the same _is_truncated_response(...) function to chat target as well keyed on finish_reason = "length"?
| # can spend the whole budget on hidden reasoning before emitting a visible answer, and a low | ||
| # limit may be a deliberate configuration. Construction preserves any completed output | ||
| # (reasoning, partial text) and falls back to a graceful empty response. | ||
| if self._is_truncated_response(response): |
There was a problem hiding this comment.
nice, I like these focused functions, makes the code readable
| incomplete_details = getattr(response, "incomplete_details", None) | ||
| reason = getattr(incomplete_details, "reason", None) if incomplete_details else None |
There was a problem hiding this comment.
My only issue with this is that if the field ever changes, it is hard to catch. Is there a way for us to switch Any to a properly typed object?
| choice = response.choices[0] if getattr(response, "choices", None) else None | ||
| if choice is not None and getattr(choice, "finish_reason", None) == "length": |
There was a problem hiding this comment.
Can we add the same _is_truncated_response(...) function to chat target as well keyed on finish_reason = "length"?
| return construct_response_from_request( | ||
| request=request, | ||
| response_text_pieces=[""], | ||
| response_type="text", | ||
| error="empty", | ||
| ) | ||
| raise EmptyResponseException(message="Failed to extract any response content.") | ||
|
|
||
| # Capture token usage from the API response and store in the first piece's metadata | ||
| capture_token_usage(pieces=pieces, response=response) |
There was a problem hiding this comment.
Here a truncated response with partial content falls through to capture_token_usage, but the truncated-empty case returns before we capture anything. I think that's the case where usage is most valuable which means the whole max_completion_tokens budget was spent (probably on reasoning) with no visible response text, and usage is populated on length-finish responses so we can actually recover it.
Also capture_token_usage already no-ops when there's no usage or no pieces so capturing on the empty path is easy thing to do for compatible providers that omit usage:
if choice is not None and getattr(choice, "finish_reason", None) == "length":
empty_message = construct_response_from_request(
request=request,
response_text_pieces=[""],
response_type="text",
error="empty",
)
capture_token_usage(pieces=empty_message.message_pieces, response=response)
return empty_message| assert "finish_reason='length'" in caplog.text | ||
|
|
||
|
|
||
| async def test_construct_message_length_empty_returns_graceful_empty( |
There was a problem hiding this comment.
Do you mind adding assertion for usage capture here?
| return construct_response_from_request( | ||
| request=request, | ||
| response_text_pieces=[""], | ||
| response_type="text", | ||
| error="empty", | ||
| ) |
There was a problem hiding this comment.
One other comment here is that we are stripping away the truncation reason. Anything downstream cannot tell whether this is a normal empty message or it is a truncated message, that's why it is empty. This might be useful to know for Scorers or Attacks. Maybe we could add something like piece.prompt_metadata["truncated"] = True
Description
Reasoning models spend completion tokens on hidden reasoning before emitting any visible output. When
max_completion_tokensis set too low, the API can returnfinish_reason="length"with empty visible content.OpenAIChatTarget._validate_responsetreated that empty content as a hard failure and raisedEmptyResponseException(surfaced with a misleading "Status Code: 204"), which also triggered the full retry loop even though retrying with the same token budget cannot succeed.Since a low token budget can be a deliberate configuration choice, this changes the behavior to warn rather than raise:
finish_reason="length", log a warning explaining that reasoning models consume tokens on hidden reasoning and thatmax_completion_tokensmay need to be increased.error="empty") so the run continues instead of raising and retrying.EmptyResponseExceptionas before.The final stored result for the empty case is unchanged from today (an
error="empty"piece). The difference is no exception, no retry storm, and a clear warning pointing at the token limit.Tests and Documentation
test_validate_response_success_lengthto assert the truncation warning is emitted when content is present.test_validate_response_length_empty_returns_empty_responseasserting that a truncated-and-empty response returns aMessagewitherror="empty"and raises nothing.tests/unit/prompt_target/target/test_openai_chat_target.pysuite passes (101 tests).No documentation changes needed. JupyText not run (no doc or notebook changes).